Skip to content

[py] Lint Python with ruff #15811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 29, 2025
Merged

[py] Lint Python with ruff #15811

merged 1 commit into from
May 29, 2025

Conversation

p0deje
Copy link
Member

@p0deje p0deje commented May 28, 2025

Use Ruff directly to format and lint Python source code files.

@p0deje p0deje requested a review from cgoldberg May 28, 2025 23:59
@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels May 28, 2025
cgoldberg
cgoldberg reviewed May 29, 2025
View reviewed changes
Copy link
Contributor

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to add --exit-non-zero-on-fix to check, and --exit-non-zero-on-format to format? We want the CI job to fail if it hits any violations.

Other than that, LGTM

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

Do you need to add --exit-non-zero-on-fix to check, and --exit-non-zero-on-format to format

Our CI job runs the format and then checks for git diff, so it will fail.

@cgoldberg cgoldberg mentioned this pull request May 29, 2025
Closed
@cgoldberg
Copy link
Contributor

@p0deje There are some issues that ruff won't auto-fix (like lines that are too long)... so git diff wouldn't see those. You might want to add those args to make sure it fails.

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

@cgoldberg Hmm, I assumed that when Ruff cannot fix/format, it would fail with non-zero exit code. At least this is how I read the flag name. Will update.

@cgoldberg
Copy link
Contributor

I'm not 100% sure what the exit code is when it can't fix... I'd have to try, but I suppose it doesn't hurt to add the flags.

@p0deje
Copy link
Member Author

p0deje commented May 29, 2025

I did some testing and ruff seems to exit non-zero when it cannot autofix:

$ ruff check --fix scripts/pinned_browsers.py; echo $status
scripts/pinned_browsers.py:208:5: E741 Ambiguous variable name: `l`
    |
206 |     mac = None
207 |     mac_hash = None
208 |     l = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…
    |     ^ E741
209 |
210 |     for data in all_data:
    |

scripts/pinned_browsers.py:208:5: F841 Local variable `l` is assigned to but never used
    |
206 |     mac = None
207 |     mac_hash = None
208 |     l = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…
    |     ^ F841
209 |
210 |     for data in all_data:
    |
    = help: Remove assignment to unused variable `l`

Found 2 errors.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).
1

For format, there seem to be no cases when it would exit non-zero and not format files.

I am going to merge this as-is, we should be safe even w/o the flags.

@p0deje p0deje merged commit 001b95b into trunk May 29, 2025
21 checks passed
@p0deje p0deje deleted the ruff-lint branch May 29, 2025 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgoldberg cgoldberg cgoldberg approved these changes

Assignees
No one assigned
Labels
B-build Includes scripting, bazel and CI integrations C-py Python Bindings
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@p0deje @cgoldberg @selenium-ci